Skip to content

lkl tools: add symlink support to cptofs#294

Merged
tavip merged 1 commit intolkl:masterfrom
copumpkin:cptofs-symlinks
Jan 16, 2017
Merged

lkl tools: add symlink support to cptofs#294
tavip merged 1 commit intolkl:masterfrom
copumpkin:cptofs-symlinks

Conversation

@copumpkin
Copy link
Copy Markdown

@copumpkin copumpkin commented Jan 16, 2017

This should fix #291. I tried to replicate the style used in cptofs.c but this is my first commit to the project so please let me know if I missed something!

I was tempted to make stat_src just take a struct stat pointer rather than adding another pointer parameter, but then I'd have to copy the fields from lkl_stat into it and this seemed slightly cleaner.


This change is Reviewable

@lkl-jenkins
Copy link
Copy Markdown

Can one of the admins verify this patch?

@copumpkin
Copy link
Copy Markdown
Author

copumpkin commented Jan 16, 2017

Actually, I'm seeing some weirdness with cpfromfs. Will investigate and possibly update the PR.

Edit: simple typo in the stat_src function; should be fixed now

@tavip
Copy link
Copy Markdown
Member

tavip commented Jan 16, 2017

@lkl-jenkins: test this please

@copumpkin
Copy link
Copy Markdown
Author

@tavip I couldn't actually find a test for cptofs, so I didn't add any automated tests that explicitly checked whether symlinks worked. Is there somewhere I should put such a thing?

Copy link
Copy Markdown
Member

@tavip tavip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, almost ready to merge. Could you please fix the checkpatch warnings / errors?

@tavip
Copy link
Copy Markdown
Member

tavip commented Jan 16, 2017

@copumpkin With regards to tests, if you can add something in tools/lkl/tests (maybe cptofs.sh?), it would be great !

@copumpkin
Copy link
Copy Markdown
Author

@tavip can I fix checkpatch issues and then submit a separate PR for tests for cptofs? I'd prefer to keep the PRs fairly self-contained

Signed-off-by: Dan Peebles <pumpkin@me.com>
@copumpkin
Copy link
Copy Markdown
Author

Fixed checkpatch issues!

@tavip
Copy link
Copy Markdown
Member

tavip commented Jan 16, 2017

can I fix checkpatch issues and then submit a separate PR for tests for cptofs? I'd prefer to keep the PRs fairly self-contained

Of course, that is the way I would prefer it too. Thanks!

@tavip
Copy link
Copy Markdown
Member

tavip commented Jan 16, 2017

@lkl-jenkins: test this please

@copumpkin
Copy link
Copy Markdown
Author

(as an aside, is there an LKL IRC channel or other chat community somewhere I can go idle?)

@tavip tavip merged commit 6c7ad3e into lkl:master Jan 16, 2017
@copumpkin
Copy link
Copy Markdown
Author

Thanks!

@Rondom
Copy link
Copy Markdown

Rondom commented Jan 16, 2017

@copumpkin There is a mailing list. Besides that @thehajime and I idle in #lkl at FreeNode, but so far we are the only ones and have not really pursued the idea of creating an "official" channel any further...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cptofs doesn't support symlinks

4 participants